-
-
Notifications
You must be signed in to change notification settings - Fork 459
Add session replay id to Sentry Logs #4740
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Performance metrics 🚀
|
Revision | Plain | With Sentry | Diff |
---|---|---|---|
b750b96 | 421.25 ms | 444.09 ms | 22.84 ms |
23d6b12 | 354.10 ms | 408.38 ms | 54.28 ms |
ee747ae | 405.43 ms | 485.70 ms | 80.28 ms |
17a0955 | 372.53 ms | 446.70 ms | 74.17 ms |
85d7417 | 347.21 ms | 394.35 ms | 47.15 ms |
d217708 | 375.27 ms | 415.68 ms | 40.41 ms |
c8125f3 | 383.82 ms | 441.66 ms | 57.84 ms |
ee747ae | 358.21 ms | 389.41 ms | 31.20 ms |
ee747ae | 554.98 ms | 611.50 ms | 56.52 ms |
ee747ae | 357.79 ms | 421.84 ms | 64.05 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
b750b96 | 1.58 MiB | 2.10 MiB | 533.20 KiB |
23d6b12 | 1.58 MiB | 2.10 MiB | 532.31 KiB |
ee747ae | 1.58 MiB | 2.10 MiB | 530.95 KiB |
17a0955 | 1.58 MiB | 2.10 MiB | 533.20 KiB |
85d7417 | 1.58 MiB | 2.10 MiB | 533.44 KiB |
d217708 | 1.58 MiB | 2.10 MiB | 532.97 KiB |
c8125f3 | 1.58 MiB | 2.10 MiB | 532.32 KiB |
ee747ae | 1.58 MiB | 2.10 MiB | 530.95 KiB |
ee747ae | 1.58 MiB | 2.10 MiB | 530.95 KiB |
ee747ae | 1.58 MiB | 2.10 MiB | 530.95 KiB |
Previous results on branch: stefanosiano/feat/replay-id-logs
Startup times
Revision | Plain | With Sentry | Diff |
---|---|---|---|
f13c41f | 343.18 ms | 409.56 ms | 66.38 ms |
9cc652c | 367.13 ms | 455.27 ms | 88.14 ms |
df60c47 | 412.76 ms | 475.58 ms | 62.82 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
f13c41f | 1.58 MiB | 2.10 MiB | 533.11 KiB |
9cc652c | 1.58 MiB | 2.10 MiB | 533.41 KiB |
df60c47 | 1.58 MiB | 2.10 MiB | 533.00 KiB |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's still a bit unclear how to handle it properly with edge-cases (e.g. buffered replay not being sent), let's follow the internal thread and implement this properly after that: https://sentry.slack.com/archives/C03USURCFBJ/p1758210278868829
added sentry._internal.replay_is_buffering to logs when replay is in buffer mode
I've seen that if the replay is in buffer mode, no replay id is set to the scope until it's captured, and then it is converted to session mode |
new SentryLogEventAttributeValue(SentryAttributeType.STRING, environment)); | ||
} | ||
|
||
final @Nullable SentryId replayId = scopes.getCombinedScopeView().getReplayId(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so I think we can simplify this by quite a bit:
- First get the replay_id from
scopes
- If it's not empty that means all good and we're in session mode (or about to enter it) - hence we don't have to send
sentry._internal.replay_is_buffering
at all - If it's empty - we try to get the replay_id from
scopes.getOptions().getReplayController().getReplayId()
- If it's not empty that means we're in buffer mode - hence we have to send
sentry._internal.replay_is_buffering: true
- If this one is still empty - that means there's no replay being recorded right now and we just don't send any of that
- If it's not empty that means we're in buffer mode - hence we have to send
- If it's not empty that means all good and we're in session mode (or about to enter it) - hence we don't have to send
This way we wouldn't have to keep replayType
in the scope either. How that sound?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@romtsn do you like it now? 😄
sentry._internal.replay_is_buffering is attached to logs if replay id is not in scope, but is in replay controller
# Conflicts: # sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/MainActivity.java
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
one nit about nullability
# Conflicts: # CHANGELOG.md
📜 Description
added session replay id to logs, if a replay is running
💡 Motivation and Context
Resolves https://linear.app/getsentry/issue/ANDROID-210/add-sentryreplay-id-to-android-logs
💚 How did you test it?
📝 Checklist
sendDefaultPII
is enabled.🔮 Next steps